[2810] Allow private config to override output directories and filename - #2811
[2810] Allow private config to override output directories and filename#2811evenmn wants to merge 9 commits into
Conversation
|
@grassesi : could you also have a look? |
|
@evenmn : could you lint please :) |
github-actions
Bot
added
infra
clessig
left a comment
clessig
left a comment
There was a problem hiding this comment.
Thanks, this is a useful feature. Some minor comments. We also should merge this into the preops branch.
Signed-off-by: evenmn <evenmn@fys.uio.no>
Signed-off-by: evenmn <evenmn@fys.uio.no>
|
@clessig Thank you for the comments. Please have a look at my answers and latest commits |
clessig
left a comment
clessig
left a comment
There was a problem hiding this comment.
Thanks for the changes! Some minor comments left. For the functions we anyway touch, we should be clear if config is "model config" or "private config". It makes the code much more readable.
| run_id: str, | ||
| mini_epoch: int | None, | ||
| model_path: str | None, | ||
| *, |
| def get_path_run(config: Config) -> Path: | ||
| """Get the current runs results_path for storing run results and logs.""" | ||
| return _get_shared_wg_path() / "results" / get_run_id_from_config(config) | ||
| return _get_path_output(config, "path_results", "results", get_run_id_from_config(config)) |
There was a problem hiding this comment.
This function doesn't make sense--we don't the path for a run. Can you remove it. The function is used in trainer.py and in train_logger.py. It's calls should be replace by calls to get_path_logs()--but please check that you agree.
|
|
||
| def get_path_model(config: Config | None = None, run_id: str | None = None) -> Path: | ||
| """Get the current runs model_path for storing model checkpoints.""" | ||
| """Get full_model_path if set, otherwise the shared per-run checkpoint directory.""" |
There was a problem hiding this comment.
config is here private config, right? Can we change the variable name.
| ext = StoreType(config.zarr_store).value # validate extension | ||
| base_path = get_path_run(config) | ||
| fname = f"validation_chkpt{mini_epoch:05d}_rank{config.rank:04d}.{ext}" | ||
| fname = config.get("output_name") or fname |
There was a problem hiding this comment.
config is here private config, right? Can we change the variable name.
github-actions
Bot
added
the
eval
2 participants
Description
Adds optional
private_configoverrides for model, log, and results directories, plus the output filename. Explicit directories are used directly, without appending the run ID. Existing defaults remain when overrides are omitted.When multiple samples are generated,
rankcan be used to separate the different samples:Other variables that can be used in the filename are
epochandstep.Checkpoint loading and logging use the selected private configuration.
Issue Number
Closes #2810
Is this PR a draft? Mark it as draft.
Checklist before asking for review
./scripts/actions.sh lint./scripts/actions.sh unit-test./scripts/actions.sh integration-testlaunch-slurm.py --time 60FastEvaluation